Skip to content

Conversation

@msvbg
Copy link

@msvbg msvbg commented Nov 2, 2025

Connections

Depends on gfx-rs/metal-rs#366.

Description
This PR supports using binding arrays of storage textures on Metal.

Testing
I have tested this change locally on my M3 MacBook with Metal 3, and my shaders are able to bind and use the binding arrays as expected.

Squash or Rebase?

Fine to squash.

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.

@msvbg msvbg force-pushed the storage_resource_binding_array branch from a5304ee to a638db8 Compare November 2, 2025 11:54
@msvbg msvbg force-pushed the storage_resource_binding_array branch from a638db8 to 9d371f3 Compare November 2, 2025 11:55
@cwfitzgerald cwfitzgerald self-assigned this Nov 5, 2025
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, some testing related issues:

  • Could you add METAL to the targets list in
    targets = "WGSL | SPIRV"
  • Could you run the tests and make sure that the binding_array_storage_buffers and parital_binding_array_storage_buffers are running and passing?
  • The issue with cargo-deny, you can add the git for metal-rs to deny.toml for now.
  • The linux failure should fix itself with a rebase/merge of main.

@msvbg
Copy link
Author

msvbg commented Nov 17, 2025

Okay after further work on this PR, I've realized that binding arrays of storage textures work (great!) but binding arrays of buffers are totally broken. Fully supporting buffers in binding arrays seems to require more work in Naga, and it feels like a bigger rabbithole than I want to go down right now. I've reduced the scope of the PR to only support storage textures in binding arrays now, not buffers. This also means no metal-rs changes.

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright looks fine - though the user won't be able to know that storage buffers aren't supported, but that's fine as a stop gap, are the tests for storage textures running/passing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants